feat: align command surface with the latest exe.dev docs - #22

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release.yml:
- Line 84: Update all three Bash steps in the release workflow that assign or
use inputs.tag_name to receive the value through the step environment, then
reference that environment variable inside the scripts instead of interpolating
inputs.tag_name directly; preserve the existing tag behavior while preventing
shell command injection.

In `@cli/README.md`:
- Around line 167-177: Correct the token-exposure guidance: in cli/README.md
lines 167-177 and cli/README.zh-CN.md lines 165-173, remove any implication that
raw exec protects tokens passed as arguments; in skills/exedev-ctl/SKILL.md
lines 42-44, remove the claim that exec or SSH prevents persisted-history
exposure; and in skills/exedev-ctl/references/exedev-ctl.md lines 272-275, state
that environment expansion avoids literal shell-history exposure but not local
process-argument exposure, unless a supported secure secret-input mechanism is
documented.

In `@k8s_cli/src/manager/parsing.rs`:
- Around line 67-71: Update parse_ssh_destinations to detect a present output
field in the outer JSON object, parse its string contents as the inner JSON
payload, and pass that payload to collect_ssh_destinations while preserving
direct-response parsing. Add a regression test covering the wrapped
{"output":"<JSON>"} response and verifying the reported ssh_dest values are
retained.

In `@scripts/release/set-version.sh`:
- Around line 30-33: Update the VERSION validation in the release script to
enforce full SemVer rules: reject leading-zero numeric components and numeric
prerelease identifiers, while accepting valid prerelease and build metadata such
as 1.2.3-rc.1+build.5. Use a complete SemVer validator or equivalent regex, and
add coverage for these accepted and rejected cases.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Findings on this change (also posted as inline comments) (4)

In scripts/release/sync-homebrew-tap.sh around line 41, address this finding:
The Homebrew sync script accepts any non-empty RELEASE_TAG and writes a formula without validating that it is a release tag/semantic version. For example, `RELEASE_TAG='not a version'` (or `v01.2.3`) is used to construct the archive filenames/URLs, and if matching assets happen to exist the script reaches `cat > "$TAP_FORMULA_PATH"`; the generated formula then contains that malformed release URL/version. This violates the requirement that malformed semantic versions fail before writing an unusable formula. The claim would be false only if an earlier guaranteed contract rejects every malformed tag before this script runs, but the script is explicitly callable with an argument/environment and performs no such check.

In scripts/release/sync-homebrew-tap.sh around line 15, address this finding:
The script accepts an arbitrary RELEASE_TAG and interpolates it into archive paths, download URLs, and double-quoted Ruby strings without validating or escaping it. A release tag such as `v1.2.3"-bad` (or any tag that does not match the semver/tag contract) can make the generated formula syntactically invalid; slash-containing tags can also make the local curl output path fail before the intended diagnostic. The release workflow validates indirectly through set-version, but this standalone sync script also supports gh/env tags and does not enforce that invariant.

In scripts/release/sync-homebrew-tap.sh around line 120, address this finding:
Formula metadata is inserted into Ruby string/class syntax without escaping or validating it. Supplying FORMULA_DESC containing `"` or a newline, or FORMULA_CLASS containing non-Ruby identifier text, produces a formula that Homebrew cannot parse (and FORMULA_DESC can inject additional formula code). The same applies to REPO_SLUG and FORMULA_LICENSE in quoted Ruby strings, so the script's configurable inputs can make formula generation succeed while producing an unusable tap file.

In cli/src/cli_command.rs around line 151, address this finding:
Raw dangerous commands do not honor the global `--yes` flag. `build_command` explicitly excludes `Commands::Exec` from `needs_server_confirmation`, so `exedev-ctl --transport http --yes exec -- team disable` sends `team disable` without `--yes`; `/exec` has no pty and the server-side confirmation cannot be answered, causing the command to hang or fail instead of honoring the documented global confirmation override. This would be disproven if raw `exec` is intentionally required to make users place `--yes` in the raw command and the global flag is documented as inapplicable to raw commands.

## Additional findings on this change (not posted inline) (8)

In k8s_cli/src/manager/process.rs around line 233, address this finding:
A hung remote command can block bootstrap without a bounded timeout.

In k8s_cli/src/manager/mod.rs around line 602, address this finding:
Bootstrap can complete while a fleet node is registered but NotReady.

In .github/workflows/release.yml around line 46, address this finding:
Manual dispatch does not constrain `tag_name` to an existing tag before checkout or release publication. `actions/checkout` receives the raw input as `ref`, so an input such as `main` or an arbitrary branch/SHA is checked out; the later version step then rejects it as non-semver, while an input that happens to be a valid semver but is not an existing tag can build from that ref and ask the publish action to create a release for a tag that was never checked out. The claim would be false only if the workflow's dispatch UI or repository permissions guaranteed that the input can only name an existing tag, which GitHub's string input does not.

In cli/src/ssh.rs around line 17, address this finding:
Raw exec is not preserved on the SSH transport when global JSON output is requested.

In core/src/shell.rs around line 52, address this finding:
Integration creation and attachment can change access/integration state without the local dangerous-command prompt.

In core/src/shell.rs around line 55, address this finding:
Several team authority changes are not covered by the dangerous-command matcher.

In k8s_cli/src/manager/process.rs around line 68, address this finding:
Remote bootstrap misclassifies generic exit status 126 as Tailnet Lock and can retry the wrong operation indefinitely until user confirmation.

In core/src/shell.rs around line 68, address this finding:
Danger matching is not anchored at subcommand boundaries, so neighboring command names can spuriously trigger confirmation.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release.yml:
- Around line 43-49: Update the Checkout step in the release workflow to set
actions/checkout’s persist-credentials option to false, while preserving the
existing ref selection and all other checkout behavior.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Unresolved from the previous review — these block approval, fix them first (1)

In k8s_cli/src/manager/process.rs, address this finding:
A hung remote command can block bootstrap without a bounded timeout.

## Findings on this change (also posted as inline comments) (1)

In k8s_cli/src/manager/parsing.rs around line 67, address this finding:
Authoritative SSH destinations are lost when `exe.dev ls` returns a JSON object with an `output` string containing the actual serialized/listed payload. `parse_vm_names` explicitly falls back to `value.output`, but `parse_ssh_destinations` only traverses the JSON object and never parses `output`; consequently the VM name may be discovered from text while its `ssh_dest` is omitted and `SshTargets::dest` uses `<vm>.exe.xyz` instead of the reported user@host destination. This would be disproven if the ls API never wraps its JSON/text output in an `output` field, despite the existing parser's explicit support for that shape for names.

## Additional findings on this change (not posted inline) (4)

In k8s_cli/src/manager/process.rs around line 233, address this finding:
Remote SSH operations have no bounded process/request lifetime. `ConnectTimeout=15` only limits connection establishment; `capture_remote_ssh_output` awaits `child.wait_with_output()` indefinitely, so a remote script that hangs (for example a package install, curl, or service operation) blocks bootstrap forever and can retain the child/pipes indefinitely. This would be disproven if an external process supervisor guaranteed a timeout for every invocation, but the shown code invokes ssh directly and has no such timeout.

In k8s_cli/src/manager/process.rs around line 251, address this finding:
The SSH retry policy can repeat unsafe bootstrap effects after an ambiguous transport failure. Any SSH exit status 255 is retried, but status 255 can occur after the remote script has already installed Tailscale/k3s or changed service state and the connection drops while returning output; the next attempt reruns the entire script. The wrapper makes scripts report status but does not make these operations transactional or idempotent (and some install/start steps have side effects), so a transient disconnect can duplicate/partially repeat bootstrap effects. This would be disproven if ssh status 255 were proven to occur only before remote execution for the deployed SSH path.

In k8s_cli/src/manager/mod.rs around line 635, address this finding:
Node taint application is additive rather than reconciliatory: it applies a desired taint with `--overwrite`, but never removes taints that are no longer in the fleet plan. If a previously isolated worker is changed to `taint: null`, rerunning bootstrap leaves the old NoSchedule taint in place; metadata therefore does not represent the plan and can keep workloads unschedulable. This would be disproven if the product intentionally guarantees that taints are immutable and stale taints are managed only by a separate command, but bootstrap's overwrite semantics and metadata obligation imply reconciliation.

In k8s_cli/src/manager/process.rs, address this finding:
Established SSH sessions have no bounded lifetime, so a bootstrap can hang indefinitely after the TCP connection succeeds.

## Previously reported and still present (3)

In scripts/release/sync-homebrew-tap.sh around line 20, address this finding:
ARCHIVE_PREFIX is accepted from the environment and interpolated into Ruby double-quoted url strings without validation, so a value containing a quote and Ruby interpolation/code can generate a formula with altered syntax or execute code when Homebrew evaluates it.

In k8s_cli/src/manager/mod.rs around line 602, address this finding:
Bootstrap proceeds to metadata and manifest deployment after nodes merely register, even if none is Ready. `wait_for_kubernetes_nodes` returns when every expected name exists but never checks `KubernetesNode.ready`; `run_bootstrap` then immediately calls `apply_node_metadata` and `kubectl_apply`. A cluster can report all nodes with Ready=False/Unknown and still receive labels, taints, and manifests, violating the readiness invariant. This would be disproven if Kubernetes registration were guaranteed to imply the intended Ready condition for every node, but k3s commonly registers nodes before readiness and the parser explicitly distinguishes `ready`.

In k8s_cli/src/manager/state.rs around line 50, address this finding:
Secret files are written with their final 0600 mode only after contents have been written. `write_secret_file` uses `fs::write` first, which creates a new file under the process umask (potentially 0644), then calls `set_permissions`; an interruption/crash between those calls leaves the kubeconfig or k3s token readable. The function also follows an existing symlink, so a path supplied via `--kubeconfig` can redirect secret contents to another file. This would be disproven if the CLI always runs with a guaranteed restrictive umask and rejects symlinked paths externally, neither of which is enforced here.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Findings on this change (also posted as inline comments) (4)

In scripts/release/sync-homebrew-tap.sh around line 140, address this finding:
Only the macOS arm64 archive is inspected for required members; the other three downloaded archives can be malformed while the script still writes a successful formula.

In .github/workflows/release.yml around line 49, address this finding:
The build can compile a different commit from the commit that triggered the release while publishing under the requested tag.

In scripts/release/sync-homebrew-tap.sh around line 91, address this finding:
Tap layout detection can select an unused formula path. When a sharded tap has not yet created `Formula/<first-letter>` for this formula, the script falls back to `Formula/<name>.rb`; conversely, a flat/custom tap containing a directory with that first-letter name is treated as sharded. In either case the new formula can be written successfully where Homebrew will not discover it.

In scripts/release/sync-homebrew-tap.sh around line 22, address this finding:
The generated formula omits `LICENSE` from `doc.install`, even though every release archive packages it and the repository's release-install documentation lists it as an archive document. Users installing through the formula therefore do not receive all published documentation/license files.

## Additional findings on this change (not posted inline) (4)

In k8s_cli/src/manager/parsing.rs around line 26, address this finding:
A wrapped serialized `ls` response is parsed inconsistently: inventory names are treated as table text instead of decoding the JSON string in `output`. For a response such as `{"output":"[{\"vm_name\":\"vm-1\",\"ssh_dest\":\"vm+vm-1@exe.dev\"}]"}`, `parse_ssh_destinations` finds the target but `parse_vm_names` falls back to `parse_vm_names_from_text`, producing a bogus name like `[{"vm_name":"vm-1",...}]` rather than `vm-1`; bootstrap then believes the real VM is missing and can issue duplicate creation (or status/destroy reports incorrect inventory). This is disproven if the exe.dev API never returns serialized JSON inside `output` for `ls`, but the changed parser explicitly supports that shape for SSH destinations and its comment says the wrapper is shared.

In k8s_cli/src/manager/mod.rs around line 743, address this finding:
Bootstrap accepts empty required credentials/endpoints because `require_env` only calls `env::var` and does not reject an empty string. In new mode, `TS_AUTHKEY=` reaches `tailscale up --auth-key ''`; in existing mode, empty `K3S_URL`/`K3S_TOKEN` pass the initial checks and are embedded in the worker install script. This allows the command to pass confirmation and create VMs before failing remotely (or potentially invoke k3s with an empty endpoint/token), leaving a partially bootstrapped fleet instead of rejecting invalid configuration up front. The claim is false only if the environment-loading layer guarantees these variables can never be present-but-empty, which ordinary process environments do not.

In scripts/release/set-version.sh, address this finding:
set-version.sh can leave the workspace partially rewritten when any later operation fails. It commits each manifest with mv as it goes, so a missing/invalid later member or path dependency (and any cargo update failure) leaves earlier manifests at the new version while the remaining manifests/lockfile remain old; mv failures also leave the per-file .tmp behind. This violates the migration/error-handling requirement that failures not leave partial temporary files or inconsistent workspace state. For example, if k8s_cli/Cargo.toml is absent or `cargo update --workspace` cannot reach the registry, core/Cargo.toml and cli/Cargo.toml have already been changed. This is introduced by the new sequential rewrite implementation; it would be disproven only if the script were always run in a disposable checkout and no caller relies on the checkout afterward, but the workflow invokes it in the build checkout and the script is independently callable.

In .github/workflows/release.yml around line 166, address this finding:
A mutable third-party action reference is executed with the release job's write-capable token, so a compromise or retagging of softprops/action-gh-release@v3 can modify repository releases (and potentially other contents allowed by that token).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release.yml:
- Around line 46-52: Update the release workflow to resolve a workflow_dispatch
tag to one immutable commit SHA before the matrix runs, then pass that SHA to
every matrix checkout instead of resolving the mutable tag independently. Ensure
the publish job verifies the tag still points to the built SHA before invoking
the release action, while preserving the existing github.sha behavior for push
events.

In `@k8s_cli/src/manager/state.rs`:
- Around line 59-73: Update the file replacement flow around fs::remove_file and
OpenOptions to write contents to a unique 0600 temporary file in the same
directory, then atomically rename it over path only after write_all succeeds.
Preserve the existing file when temporary-file creation or writing fails, clean
up any failed temporary file, and add a failure-path test verifying the original
contents remain intact.

In `@scripts/release/set-version.sh`:
- Around line 112-117: Update the manifest replacement flow around the TARGETS
backup and mv loops to track successful backup creation and whether the lockfile
refresh completed. On EXIT before a successful refresh, restore every available
target backup before deleting temporary files or backups, while preserving
normal cleanup after a successful apply.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Findings on this change (also posted as inline comments) (4)

In scripts/release/set-version.sh around line 122, address this finding:
Version synchronization rewrites the release lockfile by updating all workspace dependency selections, so a tag's build is not reproducible from its checked-in Cargo.lock and may incorporate unreviewed dependency changes.

In k8s_cli/src/manager/process.rs around line 361, address this finding:
An untrusted reported SSH destination can be interpreted as an ssh option because it is placed directly before the command without ending option parsing.

In k8s_cli/src/manager/state.rs around line 72, address this finding:
Secret-file replacement is not failure-safe: `write_secret_file` unlinks the old secret before creating the replacement and writes directly into the new path. If `write_all` is interrupted or returns an I/O error after a partial write (or the process crashes during the write), the path is left containing a truncated/partial secret; on the next run `read_or_create_k3s_token` accepts that content as the persisted token, and a kubeconfig can likewise be left incomplete. This violates the requirement to leave no unsafe intermediate or stale-secret state. The claim would be false only if the filesystem/environment guarantees `write_all` is all-or-nothing and the process cannot terminate between unlink/create/write, which normal local filesystems do not guarantee.

In k8s_cli/src/manager/state.rs around line 66, address this finding:
A failed write can leave a truncated secret that is later accepted as authoritative state. `write_secret_file` unlinks the old file before writing and never removes a partially written new file when `write_all` fails; on the next invocation `read_or_create_k3s_token` only checks `path.exists()` and returns the file contents without validating token completeness. For example, a disk-full/error-after-N-bytes while replacing `k3s-token` leaves a short token, which a subsequent bootstrap uses for the server/agents instead of regenerating or preserving the last good token. The same non-atomic behavior leaves a truncated kubeconfig that callers then use. This would be false only if the filesystem guaranteed `write_all` is all-or-nothing for these files or callers never retry/use the state after an I/O error.

## Additional findings on this change (not posted inline) (9)

In k8s_cli/src/manager/process.rs around line 277, address this finding:
A transport-status-255 retry can rerun a non-idempotent bootstrap after the remote side has already executed it. `capture_remote_ssh_output` retries solely from `output.status.code() == Some(255)` and ignores whether stdout already contains the wrapper's `__EXEDEV_K8S_EXIT__:` marker. For example, sshd runs the k3s install through the wrapper, the VM completes and starts k3s, then the client loses the connection before/while receiving the final output; ssh exits 255 and the loop sends the full install script again. This violates the no-duplicate-state-changing-step invariant; the retry is safe only when the exchange establishes that the remote command did not complete (or the operation is explicitly idempotent). This would be disproven if the SSH/remote wrapper contract guaranteed status 255 is emitted only before remote script execution, or if all retried scripts were proven idempotent.

In k8s_cli/src/manager/state.rs, address this finding:
`read_or_create_k3s_token` follows a symlink when loading an existing generated token (`path.exists()` followed by `fs::read_to_string(&path)`). An attacker or another local process that can alter `.exedev-k8s/<cluster>/k3s-token` can replace it with a symlink to an arbitrary readable file, causing bootstrap to use that file's contents as the k3s token (and potentially disclose/use unrelated secret material). The write path removes a symlink, but the read path has no `symlink_metadata`/no-follow validation, so the stated replacement-and-secret-file invariant is incomplete. This would be false only if the state directory and all entries were guaranteed immutable/trusted for the entire process lifetime.

In .github/workflows/release.yml around line 175, address this finding:
The release is not bound to the tag's commit for the entire run: after checkout pins a push run to `github.sha` (and a dispatch run initially resolves the tag), publish later passes only the tag name to the release action. If the tag is moved between checkout and publish, the archives are built from the originally triggered commit but are attached to the tag/release at its new commit, so the release's source tag and binaries disagree.

In k8s_cli/src/manager/parsing.rs around line 147, address this finding:
Text fallback drops legitimate VM names beginning with `name`, causing inventory inconsistency and duplicate-create attempts.

In k8s_cli/src/manager/parsing.rs around line 41, address this finding:
A valid JSON response with no recognized VM container is incorrectly parsed as a VM name from its serialized representation.

In k8s_cli/src/manager/mod.rs, address this finding:
A successful kubectl node probe with malformed JSON aborts bootstrap immediately instead of consuming the configured readiness retries and preserving the last probe error.

In k8s_cli/src/manager/process.rs around line 194, address this finding:
The readiness probe's `--request-timeout=8s` only bounds the Kubernetes HTTP request, not the local kubectl subprocess. `capture_command_output` awaits `TokioCommand::output()` without a wall-clock timeout or kill-on-drop, so an auth plugin, kubeconfig exec plugin, DNS/TLS path, or wedged kubectl can hang one polling attempt forever and prevent both the retry window and final diagnostics. This would be false only if kubectl is guaranteed to exit for every supported kubeconfig/plugin/network failure.

In k8s_cli/src/manager/process.rs around line 74, address this finding:
`remote_run` treats every remote status 126 as Tailnet Lock authorization, even when the 126 came from an unrelated bootstrap command. The wrapper only returns an integer marker, and the caller branches solely on `output.status == TAILNET_LOCK_AUTH_REQUIRED_STATUS`; k3s/tailscale shell commands or privilege/tool execution can independently return 126. In that case bootstrap pauses for a misleading Tailnet confirmation and, after confirmation, reruns the entire state-changing step rather than reporting the real command failure, potentially duplicating installation/service actions. This would be false only if every possible command in every generated step were guaranteed never to return 126 except `CHECK_TAILNET_LOCK_SCRIPT`.

In core/src/shell.rs around line 43, address this finding:
Several newly represented destructive/access/credential operations bypass the local confirmation guard: `share remove`, `share remove-link`, `share set-private`, `domain add`, `ssh-key add`, and `ssh-key generate-api-key` do not match any dangerous prefix (only domain rm, share set-public/add-link/access allow, and ssh-key remove are covered). For example, `exedev-ctl ssh-key generate-api-key --exp 30d` creates a credential without a prompt, and `exedev-ctl domain add vm example.com` changes domain ownership/certificate state without a prompt. This violates the stated coverage for credential-bearing, access-granting, and domain operations; it would be false only if the server treated all of these as read-only/non-authorizing operations.

## Previously reported and still present (1)

In .github/workflows/release.yml around line 52, address this finding:
A push-triggered release can publish archives built from one commit under a tag that resolves to a different commit.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@k8s_cli/src/manager/state.rs`:
- Around line 58-80: In k8s_cli/src/manager/state.rs:58-80, replace predictable
staging-path creation and pathname rename with directory-handle operations, an
unpredictable staging name, and protected no-follow semantics so an attacker
cannot substitute the staged entry; in k8s_cli/src/manager/state.rs:102-111,
open the target without following symlinks, validate metadata from that opened
descriptor, and read from the same descriptor rather than reopening the
replaceable path.

---

Outside diff comments:
In `@k8s_cli/src/manager/parsing.rs`:
- Around line 32-38: Update the JSON-parsing branch in the VM-name parsing
function to return Ok(names) immediately after successful serde_json::from_str
and collect_vm_names_from_json, regardless of whether names is empty; only fall
back to parse_vm_names_from_text when JSON parsing fails.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Unresolved from the previous review — these block approval, fix them first (2)

Somewhere in the code under review, address this finding:
`read_or_create_k3s_token` follows a symlink when loading an existing generated token (`path.exists()` followed by `fs::read_to_string(&path)`). An attacker or another local process that can alter `.exedev-k8s/<cluster>/k3s-token` can replace it with a symlink to an arbitrary readable file, causing bootstrap to use that file's contents as the k3s token (and potentially disclose/use unrelated secret material). The write path removes a symlink, but the read path has no `symlink_metadata`/no-follow validation, so the stated replacement-and-secret-file invariant is incomplete. This would be false only if the state directory and all entries were guaranteed immutable/trusted for the entire process lifetime.

In k8s_cli/src/manager/process.rs, address this finding:
A transport-status-255 retry can rerun a non-idempotent bootstrap after the remote side has already executed it. `capture_remote_ssh_output` retries solely from `output.status.code() == Some(255)` and ignores whether stdout already contains the wrapper's `__EXEDEV_K8S_EXIT__:` marker. For example, sshd runs the k3s install through the wrapper, the VM completes and starts k3s, then the client loses the connection before/while receiving the final output; ssh exits 255 and the loop sends the full install script again. This violates the no-duplicate-state-changing-step invariant; the retry is safe only when the exchange establishes that the remote command did not complete (or the operation is explicitly idempotent). This would be disproven if the SSH/remote wrapper contract guaranteed status 255 is emitted only before remote script execution, or if all retried scripts were proven idempotent.

## Findings on this change (also posted as inline comments) (5)

In .github/workflows/release.yml around line 206, address this finding:
The tag recheck is check-then-use rather than an atomic publish target. A maintainer or attacker who moves the release tag after `Verify the tag still points at the built commit` completes but before `action-gh-release` resolves `tag_name`, can cause the release to be attached to the moved tag while the archives remain from BUILT_SHA; the workflow will not refuse that move.

In core/src/shell.rs around line 78, address this finding:
Creating a team pool is not classified as dangerous even though `pool new` mutates the team's reserved-capacity resource (and the command is documented as reserved capacity). The dangerous list covers `pool delete` but has no `pool new` prefix, so `exedev-ctl pool new ...` bypasses the confirmation required for newly supported resource-mutating operations. This would be false only if pool creation is guaranteed not to reserve/bill capacity and is intentionally considered harmless.

In .github/workflows/release.yml around line 41, address this finding:
Manual dispatch does not reject an invalid tag name at the resolve boundary. Any existing non-semver tag (or a tag object whose peeled object is not a commit) is accepted and emitted as `needs.resolve.outputs.tag/sha`; the workflow only discovers a semver failure later in each matrix build when set-version runs, wasting builds and violating the requirement that invalid dispatch tags be rejected before build. There is also no explicit validation that the fetched object type is commit after peeling.

In scripts/release/sync-homebrew-tap.sh around line 182, address this finding:
An explicitly configured `TAP_FORMULA_PATH` is accepted without any validation or confinement. The script creates its parent and truncates the path with `cat >`, so a caller-controlled value such as an absolute path, `../` path, or symlink can overwrite an arbitrary file rather than a formula under the tap; this is not prevented by the repository/path validation (which only applies to auto-discovery).

In core/src/shell.rs around line 81, address this finding:
Selecting a saved billing payment method is not guarded as dangerous. The typed `billing payment default <reference>` command changes which card will be charged, but `is_dangerous` only covers `billing payment remove`; therefore an interactive invocation executes without the required local confirmation. This would be false only if the server guarantees that changing the default payment method is harmless/non-consequential and is intentionally excluded from dangerous operations.

## Additional findings on this change (not posted inline) (8)

In k8s_cli/src/manager/state.rs around line 14, address this finding:
Generated state paths can escape the intended .exedev-k8s directory because cluster.name is only checked for nonempty and is joined directly into the path; a fleet cluster name such as ../../outside causes token and generated kubeconfig writes outside the state root.

In .github/workflows/release.yml around line 79, address this finding:
The release workflow does not satisfy immutable action pinning for the build and artifact steps: checkout, toolchain installation, cache, artifact upload, and artifact download all use mutable tags (`@v7`, `@stable`, `@v2`, `@v8`). A retagged or compromised upstream action can therefore execute code in the release workflow (and in build jobs that compile third-party crates), despite the publish action itself being pinned.

In k8s_cli/src/manager/process.rs around line 188, address this finding:
A cancelled or otherwise dropped kubectl mutation can leave the kubectl child running because run_command does not set kill_on_drop and has no enclosing timeout.

In k8s_cli/src/manager/state.rs around line 35, address this finding:
Reusing a persisted token does not enforce restrictive permissions: the existing-file branch only reads the file and returns its contents, so a token file created or changed to mode 0644 remains readable by other users on every subsequent run.

In k8s_cli/src/manager/scripts.rs, address this finding:
The generated k3s server kubeconfig is made world-readable on the VM: both server installation paths pass --write-kubeconfig-mode 644, although /etc/rancher/k3s/k3s.yaml contains client credentials. Any local VM user can read those credentials before fetch_kubeconfig copies the file.

In k8s_cli/src/manager/mod.rs, address this finding:
Existing-mode readiness, metadata, and manifest operations are not bound to K3S_URL: when --kubeconfig and KUBECONFIG are absent, the code proceeds with kubectl's default context. This can report readiness for or mutate an unrelated cluster while workers were installed against the required K3S_URL.

In k8s_cli/src/manager/process.rs around line 289, address this finding:
A remote SSH stdin write failure is discarded whenever ssh also exits unsuccessfully, so the caller reports only the child status/detail and cannot distinguish a broken stdin pipe from a transport or remote failure.

In scripts/release/set-version.sh, address this finding:
The script's `cargo update --workspace` is broader than a local package-version refresh and can re-resolve compatible registry dependencies in Cargo.lock. Thus a release run can silently change the reviewed third-party lock selection even though the script says no third-party dependency selection changes; with `--locked` builds this produces artifacts from dependency versions different from the committed lockfile.

## Previously reported and still present (3)

In k8s_cli/src/manager/state.rs around line 23, address this finding:
New-mode bootstrap accepts an explicitly empty K3S_TOKEN and uses it as the cluster credential.

In scripts/release/set-version.sh around line 137, address this finding:
On a failed or interrupted `cargo update`, the rollback restores only the manifest backups, not Cargo.lock. `cargo update --workspace` runs after manifests are moved into place, while Cargo.lock is not included in TARGETS and is never backed up; if Cargo writes a new/partial lockfile before returning nonzero or the process is interrupted, EXIT cleanup restores old manifests but leaves the lockfile at the new state, violating manifest/lock consistency on failure paths.

In k8s_cli/src/manager/parsing.rs around line 26, address this finding:
A JSON wrapper whose `output` contains a valid but empty/error serialized listing is incorrectly fed to the text parser, inventing a VM name from the JSON text.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@k8s_cli/src/manager/state.rs`:
- Around line 30-45: Update state_dir_name so distinct cluster_name values
cannot map to the same directory, while retaining readable sanitization where
possible. Encode the original UTF-8 bytes injectively or append an unambiguous
encoded suffix derived from the original name, and preserve the existing
fallback for names containing only underscores or otherwise ensure it remains
collision-free.
- Around line 67-72: The existing-token path returns empty or whitespace-only
contents from read_regular_file. In the path.exists() branch, validate the
trimmed token before restrict_secret_permissions and return, producing a clear
error for an empty value; preserve normal reuse for non-empty tokens. Add a test
covering an existing empty k3s-token file.

---

Nitpick comments:
In `@scripts/release/check-version.sh`:
- Around line 21-25: Set LC_ALL=C before the SEMVER_RE match in the
version-check flow so the [[ "$VERSION" =~ $SEMVER_RE ]] evaluation uses
ASCII-only, locale-independent character ranges. Keep the existing SemVer
pattern and validation behavior unchanged.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Unresolved from the previous review — these block approval, fix them first (3)

Somewhere in the code under review, address this finding:
Existing-mode readiness, metadata, and manifest operations are not bound to K3S_URL: when --kubeconfig and KUBECONFIG are absent, the code proceeds with kubectl's default context. This can report readiness for or mutate an unrelated cluster while workers were installed against the required K3S_URL.

Somewhere in the code under review, address this finding:
The generated k3s server kubeconfig is made world-readable on the VM: both server installation paths pass --write-kubeconfig-mode 644, although /etc/rancher/k3s/k3s.yaml contains client credentials. Any local VM user can read those credentials before fetch_kubeconfig copies the file.

In k8s_cli/src/manager/process.rs, address this finding:
A transport-status-255 retry can rerun a non-idempotent bootstrap after the remote side has already executed it. `capture_remote_ssh_output` retries solely from `output.status.code() == Some(255)` and ignores whether stdout already contains the wrapper's `__EXEDEV_K8S_EXIT__:` marker. For example, sshd runs the k3s install through the wrapper, the VM completes and starts k3s, then the client loses the connection before/while receiving the final output; ssh exits 255 and the loop sends the full install script again. This violates the no-duplicate-state-changing-step invariant; the retry is safe only when the exchange establishes that the remote command did not complete (or the operation is explicitly idempotent). This would be disproven if the SSH/remote wrapper contract guaranteed status 255 is emitted only before remote script execution, or if all retried scripts were proven idempotent.

## Findings on this change (also posted as inline comments) (5)

In .github/workflows/release.yml around line 194, address this finding:
The publish-time moving-tag check is not an atomic safeguard: it reads the tag target, then separately invokes `softprops/action-gh-release` with only `tag_name`. If an authorized actor moves the tag after the `gh api` check but before or during the release action, the action can create/update the release for the new target while attaching archives built from `BUILT_SHA`. Thus the workflow can publish artifacts that do not match the tag despite the verification step. This would be false only if the repository prevents tag updates for the entire check-to-action interval or the release action/API pins the release target to `BUILT_SHA` (neither is expressed here).

In scripts/release/sync-homebrew-tap.sh around line 175, address this finding:
Archive member validation checks only that expected names appear, not that the entries are regular root files, so a malicious or malformed release archive can pass validation while formula installation receives the wrong payload type.

In scripts/release/sync-homebrew-tap.sh around line 72, address this finding:
The script allows FORMULA_NAME and FORMULA_CLASS to identify different formulae, producing a file Homebrew cannot discover under the requested formula name.

In core/src/shell.rs around line 52, address this finding:
`share receive-email` is a mutating access/capability command but is not classified as dangerous. The CLI builds `share receive-email <vm> [on|off] [--reply-policy ...]`, which changes whether a VM can send email and who it may email; `guard_dangerous_command` is called for both transports before execution, yet `is_dangerous` has no `share receive-email` rule. Thus a user can enable broad email capability without confirmation.

In k8s_cli/src/manager/tests.rs around line 188, address this finding:
The destination-precedence test does not actually verify the required invariant that `ssh_dest` is authoritative: in `parses_ssh_destinations_from_ls_json`, each record either has matching `ssh_dest`/host fields or only host/user fields. A regression that incorrectly prefers `ssh_host` + `ssh_user` over a conflicting `ssh_dest` would pass the suite, despite connecting to a different VM route. This is a concrete test-alignment gap; it would be disproven if another exercised test (outside the reviewed file) supplies conflicting fields and asserts `ssh_dest` wins.

## Additional findings on this change (not posted inline) (8)

In .github/workflows/release.yml, address this finding:
The publish job grants `contents: write` to every step in the job, not only to the GitHub Release action. Consequently the artifact download and tag-verification shell step execute with a write-capable `github.token`; compromise of the download action or a command/tool invoked before the publish step could mutate repository contents/releases, contrary to the scope's least-privilege requirement. This would be disproven only if GitHub Actions supported per-step reduction of the job token here or the preceding steps demonstrably received a separate read-only token, neither of which this workflow configures.

In core/src/shell.rs around line 65, address this finding:
The `integrations setup ` prefix blocks the explicitly read-only `integrations setup <type> --list` form. `build_integrations_command` emits that exact form for `IntegrationSetupCmd.list`, while the classifier treats every setup invocation as dangerous; consequently a listing operation prompts and can be cancelled, violating the non-mutating-command exemption.

In k8s_cli/src/manager/parsing.rs around line 23, address this finding:
When a JSON response contains both outer listing records and an `output` field containing a serialized listing, the two parsers disagree with the full payload: `parse_vm_names` returns immediately once it finds any outer name, and `parse_ssh_destinations` only unwraps when it finds no outer destinations. Thus `{"vms":[{"vm_name":"existing"}],"output":"[{\"vm_name\":\"new\",\"ssh_dest\":\"vm+new@exe.dev\"}]"}` yields only `existing` and no target for `new`, causing inventory/name-target mappings to be incomplete and potentially causing bootstrap to recreate or use hostname fallback for a VM represented in the serialized listing. This is disproven if the API contract guarantees wrapper objects never contain any listing fields outside `output`; the stated obligation for partially populated/wrapped listings should still be covered by a test or explicit invariant.

In k8s_cli/src/manager/scripts.rs around line 87, address this finding:
The systemd agent bootstrap can report success while the k3s agent has not actually become usable: it starts the unit with `systemctl start --no-block`, and `k3s_service_started` treats `activating` as success. The readiness loop then breaks immediately and the final check also accepts the same transient state, so a subsequent startup failure or hang is not surfaced.

In .github/workflows/release.yml, address this finding:
The push path never validates the release tag before resolving it. Because the trigger pattern is only `v*`, a pushed tag such as `v1.2` or `vrelease` reaches the resolve job and is exported to all builds; each matrix job then performs checkout and toolchain setup before `set-version.sh` rejects it. This violates the stated resolve-time validation and allows workflow runs for tags outside the binary/Cargo SemVer grammar (with wasted build work and no early rejection). The claim would be false if an external repository policy guaranteed every pushed `v*` tag is valid SemVer, but the workflow itself provides no such enforcement.

In core/src/shell.rs, address this finding:
Several dangerous prefixes are matched without a token boundary, so valid/raw commands whose next token merely starts with the dangerous spelling are classified as dangerous. For example, `exec -- ssh-key generate-api-keyx` (and similarly `team disablex`, `billing capacity-report`, or `team settings auto-join oncall`) produces that exact command string and triggers the confirmation even though it is not the dangerous subcommand. This violates the invariant that a prefix should match only at a word boundary; the `starts_with(prefix)` branch currently accepts arbitrary suffixes for entries lacking a trailing space. This is a false-positive safety/CLI correctness issue, and it would be disproven if the remote command grammar intentionally treats these suffix forms as aliases of the dangerous commands.

In k8s_cli/src/manager/scripts.rs, address this finding:
Tailnet Lock lockout handling is bypassed when `tailscale up` returns the lockout error, so the script exits with the raw status before running `CHECK_TAILNET_LOCK_SCRIPT`; consequently the documented interactive sign-and-retry flow cannot trigger for the intended failure.

In core/src/shell.rs, address this finding:
The dangerous-command classifier uses unbounded `starts_with` for several prefixes that do not end in a space, so raw commands whose subcommand is only a prefix of a dangerous form are incorrectly treated as dangerous. For example, `exec -- team disablex`, `ssh-key generate-api-keyx`, or `billing capacityfoo` reaches `is_dangerous` and prompts even though these are distinct command forms and should remain outside the policy.

## Previously reported and still present (4)

In k8s_cli/src/manager/parsing.rs around line 39, address this finding:
Plain-text error/status responses are treated as VM inventory names, violating the requirement that status/error strings not become names. For example, `parse_vm_names(r#"{"output":"Error: quota exceeded\n"}"#)` falls through to `parse_vm_names_from_text`, which returns `{"Error:"}`; an unwrapped `VM vm-1 is unavailable` similarly returns `{"VM"}`. A response such as `vm-1 does not exist` can falsely mark the planned VM as present, suppress creation, and later make bootstrap SSH to `vm-1.exe.xyz`. The parser should distinguish a valid rendered table from error/status text (or otherwise reject such text). This is introduced by the text fallback behavior in this change; it would be disproven only if the exe.dev client contract guarantees every non-JSON output is always a valid table and never an error/status body.

In .github/workflows/release.yml, address this finding:
Tag verification and release publication are separated by a TOCTOU window. The workflow verifies that the tag resolves to `BUILT_SHA`, then invokes the release action using the mutable tag name; if an attacker moves the tag after the API check but before or during the action, the release can be published under a tag that no longer points to the built commit. Thus the check does not guarantee publishing is blocked when the tag moves after builds. The claim would be false only if tag mutation were prevented for the entire job by an external immutable-tag policy, which is not enforced by this workflow.

In scripts/release/sync-homebrew-tap.sh around line 121, address this finding:
An explicit formula path is not safely constrained to the tap and can overwrite an arbitrary file (or follow a symlink) despite the script's path check.

In scripts/release/sync-homebrew-tap.sh around line 106, address this finding:
Automatic layout discovery can select a sharded destination based solely on an unrelated one-character directory, even when the tap is flat and Homebrew will use the flat formula path.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

## Unresolved from the previous review — these block approval, fix them first (3)

Somewhere in the code under review, address this finding:
The tag recheck is check-then-use rather than an atomic publish target. A maintainer or attacker who moves the release tag after `Verify the tag still points at the built commit` completes but before `action-gh-release` resolves `tag_name`, can cause the release to be attached to the moved tag while the archives remain from BUILT_SHA; the workflow will not refuse that move.

Somewhere in the code under review, address this finding:
The publish job grants `contents: write` to every step in the job, not only to the GitHub Release action. Consequently the artifact download and tag-verification shell step execute with a write-capable `github.token`; compromise of the download action or a command/tool invoked before the publish step could mutate repository contents/releases, contrary to the scope's least-privilege requirement. This would be disproven only if GitHub Actions supported per-step reduction of the job token here or the preceding steps demonstrably received a separate read-only token, neither of which this workflow configures.

In k8s_cli/src/manager/process.rs, address this finding:
A transport-status-255 retry can rerun a non-idempotent bootstrap after the remote side has already executed it. `capture_remote_ssh_output` retries solely from `output.status.code() == Some(255)` and ignores whether stdout already contains the wrapper's `__EXEDEV_K8S_EXIT__:` marker. For example, sshd runs the k3s install through the wrapper, the VM completes and starts k3s, then the client loses the connection before/while receiving the final output; ssh exits 255 and the loop sends the full install script again. This violates the no-duplicate-state-changing-step invariant; the retry is safe only when the exchange establishes that the remote command did not complete (or the operation is explicitly idempotent). This would be disproven if the SSH/remote wrapper contract guaranteed status 255 is emitted only before remote script execution, or if all retried scripts were proven idempotent.

## Findings on this change (also posted as inline comments) (2)

In k8s_cli/src/manager/mod.rs around line 454, address this finding:
Endpoint comparison fails to normalize a trailing dot when an explicit port is present. `same_cluster_endpoint("https://k3s.example.:6443", "https://k3s.example:6443")` parses the first host as `k3s.example.` (the `trim_end_matches('.')` is applied to the whole authority before splitting, so it cannot remove a dot before `:6443`) and therefore rejects two equivalent DNS authorities. This violates the compatibility requirement for trailing dots and blocks existing-cluster bootstrap for a valid kubeconfig/K3S_URL pair.

In k8s_cli/src/manager/state.rs around line 169, address this finding:
Permission tightening of a reused token has a symlink-swap window that can chmod an attacker-selected target.

## Additional findings on this change (not posted inline) (6)

In k8s_cli/src/manager/mod.rs around line 291, address this finding:
Changing a node's desired taint to none (or to a different taint) leaves stale scheduling restrictions on the cluster, and status reports the node as healthy anyway. `apply_node_metadata` only issues `kubectl taint` when the plan has a taint, so it never removes an old taint; meanwhile `print_kubernetes_status` treats every taint as acceptable when `expected.taint` is `None` and only checks presence rather than exact taint state. For example, a node previously configured with `exedev.dev/pool=blue:NoSchedule` remains tainted after the fleet is edited to unisolated, while `status` prints `taint=ok`, violating metadata reconciliation and drift reporting.

In k8s_cli/src/manager/scripts.rs, address this finding:
The supervisor path downloads and executes the k3s installer without checksum verification.

In k8s_cli/src/manager/scripts.rs around line 223, address this finding:
The no-supervisor PID fallback can incorrectly skip starting k3s when its PID file is stale or the PID has been reused by an unrelated process.

In k8s_cli/src/manager/state.rs around line 115, address this finding:
An explicit bare filename supplied as `--kubeconfig` cannot be written in the current directory.

In k8s_cli/src/manager/state.rs around line 116, address this finding:
Cluster-derived state paths can escape the intended state directory when a parent component is a symlink.

In k8s_cli/src/manager/scripts.rs around line 166, address this finding:
Failed or interrupted no-supervisor k3s downloads can leave credential-free but persistent temporary binary/hash files in /tmp because cleanup is performed only on checksum mismatch or successful completion.

## Previously reported and still present (2)

In scripts/release/set-version.sh around line 124, address this finding:
set-version.sh can leave a partially updated workspace and stale Cargo.lock when applying staged manifests is interrupted or a move fails.

In scripts/release/sync-homebrew-tap.sh around line 215, address this finding:
Archive validation only proves that at least one regular entry with each required name appears in the verbose tar listing; an archive containing a required regular file followed by a duplicate symlink or non-regular entry with the same name can pass validation while extraction/install uses the later entry.